-
Notifications
You must be signed in to change notification settings - Fork 31
#1 Add attribute references and context. #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…thodology for logs.
…ttributes-and-context
Co-authored-by: Alex Engelberg <alex.benjamin.engelberg@gmail.com>
…ttributes-and-context
|
This pull request has been linked to Shortcut Story #154351: Implement context type and attribute references.. |
| /** | ||
| * If true, the context will _not_ appear on the Contexts page in the LaunchDarkly dashboard. | ||
| */ | ||
| transient?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was in the wrong place. Originally it was _meta and it looks like I forgot to update that after it changed.
| ); | ||
| if (index < 0) { | ||
| throw new Error(`Did not find expected message: ${expectedMessage}`); | ||
| throw new Error(`Did not find expected message: ${expectedMessage} received: ${this.messages}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed this on the other PR.
| "build": "tsc" | ||
| }, | ||
| "license": "Apache-2.0", | ||
| "dependencies": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now the server common is using common.
| @@ -1,4 +1,4 @@ | |||
| import { LDContext } from './LDContext'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the context related interfaces were moved to common.
| * For now this is for testing and therefore is flagged internal. | ||
| * It will not be accessible outside this package. | ||
| * | ||
| * @internal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may change depending on where the filtering goes. Either becoming part of the interface, or getting further encapsulated. It is nice to be able to access these for a quick sanity check in the tests though.
|
|
||
| private contexts: Record<string, LDContextCommon> = {}; | ||
|
|
||
| private privateAttributeReferences?: Record<string, AttributeReference[]>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did decide to pre-process attribute references. They aren't as unbounded as doing a full conversion. I may change it to be lazy if the performance is a problem.
Co-authored-by: Matthew M. Keeler <keelerm84@gmail.com>
Co-authored-by: Matthew M. Keeler <keelerm84@gmail.com>
This adds the basic implementation of attribute references and parsing. It does not add attribute redaction for event filtering. There will be some changes once this is added.
The general approach here is somewhere between the typed and untyped SDKs. Get things uniform enough to simplify evaluation, but do not comprehensively copy and evaluate them. This is to try to reduce the work that needs to be done on the hot path.
Because this code would be common to client and server SDKs it is in
sdk-common. I also moved the validation code over so it could be used there as well as sever.